core/math/quaternion.cpp (1)

269-368: Extract the shared spherical-cubic core.

These two overloads duplicate the full phase-alignment and dual-expmap flow from spherical_cubic_interpolate{_in_time}. A small helper that takes the scalar interpolation op would keep future fixes to the quaternion math in one place.

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@core/math/quaternion.cpp` around lines 269 - 368, Both
spherical_monotonic_cubic_interpolate and
spherical_monotonic_cubic_interpolate_in_time duplicate the phase-alignment,
flip handling, dual ExpMap and slerp blending logic; extract that shared flow
into a single helper (e.g., spherical_monotonic_cubic_interpolate_core) that
accepts a scalar interpolation callable (std::function or template parameter)
and any optional time arguments, so the two public methods call the core with
either Math::monotonic_cubic_interpolate or
Math::monotonic_cubic_interpolate_in_time; update the core to perform Basis
conversions, flip adjustments (from_q, pre_q, to_q, post_q), compute ln vectors
in both from_q and to_q spaces, call the provided scalar interpolator for x/y/z,
build q1/q2 and return q1.slerp(q2, p_weight), keeping the public function names
spherical_monotonic_cubic_interpolate and
spherical_monotonic_cubic_interpolate_in_time as thin wrappers.

scene/resources/animation.cpp (1)

6566-6754: Extract the shared Variant cubic dispatcher before these two implementations drift.

Animation::monotonic_cubic_interpolate_in_time_variant is effectively a second copy of Animation::cubic_interpolate_in_time_variant. Any future type fix now has to be mirrored across another very large switch and array-walking block. I'd pull the shared traversal into a helper and inject the cubic vs monotonic primitives.

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@scene/resources/animation.cpp` around lines 6566 - 6754, The two large
switch+array traversal blocks in
Animation::monotonic_cubic_interpolate_in_time_variant and
Animation::cubic_interpolate_in_time_variant should be refactored into a single
helper (e.g., Animation::variant_cubic_dispatcher) that accepts a strategy flag
or function pointers for the primitive scalar/vector interpolation routines
(e.g., Math::cubic_interpolate_in_time vs
Math::monotonic_cubic_interpolate_in_time and the instance methods like
Vector2::monotonic_cubic_interpolate_in_time / cubic_interpolate_in_time,
Quaternion::spherical_monotonic_cubic_interpolate_in_time /
spherical_cubic_interpolate_in_time). Move the shared array handling and
type-switch logic into that helper and have the two public methods call it with
the appropriate primitive functions; ensure you preserve the special-casing
(numeric cast_to_blendwise/cast_from_blendwise, string fallback,
PACKED_BYTE_ARRAY skip, p_snap_array_element behavior) by passing any needed
flags through the helper.

doc/classes/Vector4.xml (1)

99-122: Optional: clarify monotonicity is per-component.

This avoids readers assuming path-level monotonicity for the whole vector.

Wording tweak

- Performs a monotonic cubic interpolation between this vector and [param b] using [param pre_a] and [param post_b] as handles, and returns the result at position [param weight].
+ Performs a per-component monotonic cubic interpolation between this vector and [param b] using [param pre_a] and [param post_b] as handles, and returns the result at position [param weight].

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@doc/classes/Vector4.xml` around lines 99 - 122, Update the descriptions for
the methods monotonic_cubic_interpolate and monotonic_cubic_interpolate_in_time
in Vector4 documentation to explicitly state that "monotonic" is applied
per-component (each vector component is interpolated independently), so readers
don't assume the interpolation guarantees monotonicity along the whole vector
path; mention that the monotonicity guarantee applies component-wise rather than
across the combined vector trajectory.

tests/core/math/test_math_funcs.h (1)

672-700: Add an invariant-style regression for “no overshoot”.

The current assertions are mostly snapshot values. Adding a boundedness check against keyframe endpoints would better lock in the issue #854 guarantee.

Proposed test addition

+TEST_CASE_TEMPLATE("[Math] monotonic_cubic_interpolate no_overshoot_regression", T, float, double) {
+	const T v1 = Math::monotonic_cubic_interpolate((T)90.0, (T)0.0, (T)0.0, (T)0.0, (T)0.25);
+	const T v2 = Math::monotonic_cubic_interpolate((T)90.0, (T)0.0, (T)0.0, (T)0.0, (T)0.5);
+	const T v3 = Math::monotonic_cubic_interpolate((T)90.0, (T)0.0, (T)0.0, (T)0.0, (T)0.75);
+
+	CHECK(v1 <= (T)90.0);
+	CHECK(v1 >= (T)0.0);
+	CHECK(v2 <= (T)90.0);
+	CHECK(v2 >= (T)0.0);
+	CHECK(v3 <= (T)90.0);
+	CHECK(v3 >= (T)0.0);
+}

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@tests/core/math/test_math_funcs.h` around lines 672 - 700, Add invariant "no
overshoot" checks to the existing tests for monotonic_cubic_interpolate_angle,
monotonic_cubic_interpolate_in_time, and
monotonic_cubic_interpolate_angle_in_time: for each call already made in those
TEST_CASE_TEMPLATE blocks, add an assertion that the returned value is bounded
between the start and end keyframe values (use min(start,end) and
max(start,end)); for the angle variants compute the shortest-interval normalized
endpoints (or unwrap so start/end define the intended interval) and assert the
interpolated angle lies within that unwrapped interval; use the existing doctest
CHECK macros (e.g. CHECK(result >= min && result <= max)) so the tests enforce
no-overshoot invariants in addition to the snapshot assertions.

tests/core/math/test_math_funcs.h (2)

658-670: Add a held-segment regression to the scalar helper.

The monotonic logic in core/math/math_funcs.h:832-878 matters most when a flat p_from == p_to segment would otherwise inherit slope from its neighbors. This block still only covers a rising segment plus extrapolation, so it doesn't lock the no-overshoot case behind GH-854.

Example regression to add

+ CHECK(Math::monotonic_cubic_interpolate((T)0.0, (T)0.0, (T)1.0, (T)1.0, (T)0.25) == doctest::Approx((T)0.0));
+ CHECK(Math::monotonic_cubic_interpolate((T)0.0, (T)0.0, (T)1.0, (T)1.0, (T)0.5) == doctest::Approx((T)0.0));
+ CHECK(Math::monotonic_cubic_interpolate((T)0.0, (T)0.0, (T)1.0, (T)1.0, (T)0.75) == doctest::Approx((T)0.0));

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@tests/core/math/test_math_funcs.h` around lines 658 - 670, The test suite
needs a held-segment regression: add cases to the TEST_CASE_TEMPLATE for
Math::monotonic_cubic_interpolate that exercise a flat segment (p_from == p_to)
so the scalar helper locks a zero slope instead of inheriting neighbor slopes;
then update the monotonic cubic logic in core/math/math_funcs.h (function
Math::monotonic_cubic_interpolate / the scalar helper in that file) to detect
p_from == p_to and set the segment derivatives to zero (and handle extrapolation
correctly) so flat segments do not acquire nonzero slopes from neighbors or
produce overshoot.

---

694-700: Mirror the held-angle case in _angle_in_time.

The plain _angle block above already protects a flat 0 -> 0 segment with non-zero neighbors, but this time-aware variant only checks a smooth increasing arc. Since the animation regression is about playback on flat segments, adding the same plateau case here would make this the real regression guard.

Example regression to add

+ CHECK(Math::monotonic_cubic_interpolate_angle_in_time((T)0.0, (T)0.0, (T)(Math::PI / 2.0), (T)(Math::PI / 2.0), (T)0.25, (T)0.5, (T)0.0, (T)1.0) == doctest::Approx((T)0.0));
+ CHECK(Math::monotonic_cubic_interpolate_angle_in_time((T)0.0, (T)0.0, (T)(Math::PI / 2.0), (T)(Math::PI / 2.0), (T)0.5, (T)0.5, (T)0.0, (T)1.0) == doctest::Approx((T)0.0));
+ CHECK(Math::monotonic_cubic_interpolate_angle_in_time((T)0.0, (T)0.0, (T)(Math::PI / 2.0), (T)(Math::PI / 2.0), (T)0.75, (T)0.5, (T)0.0, (T)1.0) == doctest::Approx((T)0.0));

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@tests/core/math/test_math_funcs.h` around lines 694 - 700,
monotonic_cubic_interpolate_angle_in_time is missing the plateau/held-angle
guard that _angle_in_time has; add the same early-return case to detect a flat
segment (e.g., when start and end angles produce a zero delta across the
interval while neighbors are non-zero) and return the held angle so playback on
flat segments remains stable; update the logic in
monotonic_cubic_interpolate_angle_in_time (the time-aware interpolation routine)
to mirror the held-angle protection present in _angle_in_time so 0->0 segments
with non-zero neighbors are preserved.

doc/classes/Animation.xml (1)

744-746: Add the angle-mode normalization note for consistency.

Line 744 introduces another *_ANGLE interpolation type, but unlike Line 733-740 it doesn’t mention normalization behavior. If behavior matches other angle interpolations, documenting that caveat here avoids confusion.

📘 Suggested doc tweak

 		<constant name="INTERPOLATION_CUBIC_MONOTONIC_ANGLE" value="6" enum="InterpolationType">
 			Monotonic Cubic interpolation with shortest path rotation.
+			[b]Note:[/b] The result value is always normalized and may not match the key value.
 		</constant>

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@doc/classes/Animation.xml` around lines 744 - 746, Add the same
angle-normalization note to the INTERPOLATION_CUBIC_MONOTONIC_ANGLE constant
documentation: state that this interpolation uses shortest-path rotation and
input/output angles are normalized (e.g., wrapped to -180..180 or consistent
internal angle range) prior to interpolation, mirroring the wording used for the
other *_ANGLE entries; update the <constant
name="INTERPOLATION_CUBIC_MONOTONIC_ANGLE"> description to include that caveat
so behavior is consistent with the other angle interpolation docs.